[18.0][MIG] website_sale_charge_payment_fee: Migration to version 18.0#1123
[18.0][MIG] website_sale_charge_payment_fee: Migration to version 18.0#1123eduezerouali-tecnativa wants to merge 18 commits intoOCA:18.0from
Conversation
b4062d4 to
541f2ef
Compare
| with RecordCapturer(self.env["sale.order"], []) as capture: | ||
| self.start_tour("/shop", "payment_fee_tour", login="portal", step_delay=100) | ||
| created_order = capture.records | ||
| price = 10 / 100 * 99.0 |
There was a problem hiding this comment.
Why does this value change?
There was a problem hiding this comment.
It does not change the value, actually is calculate the expected value of created_order.amount_payment_fee. That's why next step it and assetEqual.
There was a problem hiding this comment.
I mean, why does the calculation change? Before, it was done with price = 10 / 100 * 49.5
There was a problem hiding this comment.
Could you avoid depending on demo data to prevent external changes from altering the behavior here?
| res = super()._compute_website_order_line() | ||
| website_order_line = self.env["sale.order.line"] | ||
| for order in self: | ||
| for line in order.website_order_line: | ||
| if not line.payment_fee_line: | ||
| website_order_line |= line | ||
| order.website_order_line = website_order_line | ||
| return res |
There was a problem hiding this comment.
Why change this code? I think the older code is clearer and more compact.
| "is_published": True, | ||
| } | ||
| ) | ||
| transfer_provider._compute_charge_fee_description() |
There was a problem hiding this comment.
You don’t need the _compute_charge_fee_description function; it should be called automatically.
| amount_payment_fee = 0.0 | ||
| if ( | ||
| self.env["website"] | ||
| .get_current_website() | ||
| .show_line_subtotals_tax_selection | ||
| == "tax_excluded" | ||
| ): | ||
| for line in order.order_line: | ||
| if line.payment_fee_line: | ||
| amount_payment_fee += line.price_subtotal | ||
| else: | ||
| for line in order.order_line: | ||
| if line.payment_fee_line: | ||
| amount_payment_fee += line.price_total |
There was a problem hiding this comment.
The same here: Why change this code? I think the older code is clearer and more compact.
| browser.location.href = | ||
| "/shop/payment?provider_id=" + | ||
| providerId + | ||
| "&payment_option_id=" + | ||
| paymentOptionId; | ||
| } |
There was a problem hiding this comment.
I attached a video. I believe the problem occurs when you have two or more payment providers. In this case, I installed Demo and Wire Transfer and selected the second payment option. The window flickers, and internally the sale order changes when another payment method is selected. So, I think the window is not being refreshed correctly.
website_sale_charge_payment_fee.mp4
There was a problem hiding this comment.
@carlos-lopez-tecnativa @eduezerouali-tecnativa we added this function in the migration to resolve the problem, could you add and test, thanks:
_prepareTransactionRouteParams() {
const transactionRouteParams = this._super(...arguments);
// eslint-disable-next-line no-undef
const cartValueEl = document.querySelector(".oe_currency_value");
const cartValue = cartValueEl?.textContent || "0";
transactionRouteParams.amount = Number(cartValue);
return transactionRouteParams;
},
There was a problem hiding this comment.
@Alexgars73 That problem was already solve or you are talking about express checkout?
There was a problem hiding this comment.
@eduezerouali-tecnativa i'm getting the same error that @carlos-lopez-tecnativa mentioned "The cart has been updated. Please refresh the page".
There was a problem hiding this comment.
@Alexgars73 try on runboat, it is working fine.


e9cb772 to
1d1663f
Compare
|
@carlos-lopez-tecnativa thanks for your review! changes you requested done. Couldn't replicate what your error. Could you give more details about that? |
| order.amount_payment_fee = sum( | ||
| order.order_line.filtered("payment_fee_line").mapped("price_total") | ||
| order.order_line.filtered("payment_fee_line").mapped( | ||
| "price_subtotal" |
There was a problem hiding this comment.
Why price_subtotal??
There was a problem hiding this comment.
It should be price_total
| charge_fee_description = fields.Text( | ||
| "Fee Description", compute="_compute_charge_fee_description" | ||
| ) | ||
| charge_fee_product_id = fields.Many2one("product.product", string="Fee Product") |
There was a problem hiding this comment.
I think this product should have a domain that allows selecting only records of type service. What do you think?
There was a problem hiding this comment.
I agree with you. Makes sense, as I can't see a possible scenario where that fee could be a good
| browser.location.href = | ||
| "/shop/payment?provider_id=" + | ||
| providerId + | ||
| "&payment_option_id=" + | ||
| paymentOptionId; | ||
| } |
There was a problem hiding this comment.
I attached a video. I believe the problem occurs when you have two or more payment providers. In this case, I installed Demo and Wire Transfer and selected the second payment option. The window flickers, and internally the sale order changes when another payment method is selected. So, I think the window is not being refreshed correctly.
website_sale_charge_payment_fee.mp4
1d1663f to
b1edf83
Compare
carlos-lopez-tecnativa
left a comment
There was a problem hiding this comment.
Working fine, just a doubt about the class change. I just want to understand the change. Other than that, LGTM.
| with RecordCapturer(self.env["sale.order"], []) as capture: | ||
| self.start_tour("/shop", "payment_fee_tour", login="portal", step_delay=100) | ||
| created_order = capture.records | ||
| price = 10 / 100 * 99.0 |
There was a problem hiding this comment.
Could you avoid depending on demo data to prevent external changes from altering the behavior here?
| @http.route( | ||
| "/shop/payment", type="http", auth="public", website=True, sitemap=False | ||
| ) | ||
| def shop_payment(self, **post): |
There was a problem hiding this comment.
This class PaymentPortal, doesn’t have the shop_payment function in the payment module. This method is added in the website_sale module. Why did you change the class to inherit from it instead of inheriting from the WebsiteSale class?
b1edf83 to
b9e382d
Compare
|
@carlos-lopez-tecnativa Changes done, no it does not depends on demo data for test. |
b9e382d to
d20d908
Compare
|
@eduezerouali-tecnativa I tested and it is not working well when you try to pay directly with X payment. |
| <field name="inherit_id" ref="payment.payment_provider_form" /> | ||
| <field name="arch" type="xml"> | ||
| <xpath expr="//notebook" position="inside"> | ||
| <page string="Charge payment fee"> |
There was a problem hiding this comment.
@eduezerouali-tecnativa I tested and it is not working well when you try to pay directly with X payment.
@Reyes4711-S73 The case you mean is when the payment provider supports express checkout, but in this case, it’s out of scope for this module.
@eduezerouali-tecnativa You can add it to the ROADMAP and hide the page in this case.
I don’t know which payment providers support express checkout apart from Demo, which is never used in the real world.
| <page string="Charge payment fee"> | |
| <page string="Charge payment fee" invisible="allow_express_checkout"> |
There was a problem hiding this comment.
I implemented your suggestions, anyways i think that stripe used it too. Also add it to roadmap. Thanks :)
| ) | ||
| cls.product_product_optional = cls.env["product.template"].create( | ||
| { | ||
| "name": "Oprional", |
There was a problem hiding this comment.
| "name": "Oprional", | |
| "name": "Optional", |
| "is_published": True, | ||
| "sale_ok": True, | ||
| "taxes_id": [Command.clear()], | ||
| "optional_product_ids": [Command.set(cls.product_product_optional.ids)], |
There was a problem hiding this comment.
Is the optional product really necessary? Why?
There was a problem hiding this comment.
not really necessary, i did that,just to not adjust the tour. Anyways i did adjust the tour. :)
d20d908 to
9c7ddf1
Compare
|
/ocabot migration website_sale_charge_payment_fee |
|
@pilarvargas-tecnativa please could you review |
ADD website_sale_charge_payment_fee_delivery: link module ADD website_sale_charge_payment_fee_quote: link module
…o acquirer If there are no acquirers, gets ``` Error to render compiling AST TypeError: 'NoneType' object has no attribute '__getitem__' Template: 929 Path: /templates/t/t/div/div[1]/div/div[2]/input Node: <input type="hidden" t-att-value="selected_acquirer.id if selected_acquirer else acquirers[0].id" name="selected_acquirer_id" data-oe-id="1278" data-oe-xpath="/data/xpath[3]/input" data-oe-model="ir.ui.view" data-oe-field="arch"/> ```
…re is no acquirer
We avoid this way the warning on runbot and it's more tolerant to inheritance.
Currently translated at 100.0% (21 of 21 strings) Translation: e-commerce-14.0/e-commerce-14.0-website_sale_charge_payment_fee Translate-URL: https://translation.odoo-community.org/projects/e-commerce-14-0/e-commerce-14-0-website_sale_charge_payment_fee/es/
9c7ddf1 to
3552f9e
Compare








cc @Tecnativa TT58475
ping @pilarvargas-tecnativa @carlos-lopez-tecnativa
Migrated from #1047
Supersedes #1047